Conversation
P0 Security: - Fix XSS in loadPrompts() - escape prompt.name, content, tags with escapeHtml() - Fix XSS in showEditPromptModal() - use DOM APIs instead of string interpolation - Remove unused windows crate from root Cargo.toml (service has its own) P1 Features: - Add service thread panic protection with auto-restart (catch_unwind + 3s retry) - Add prompt export (export_prompts command + download JSON) - Add prompt import (import_prompts command + file picker UI) - Add main panel search (search_prompts command + search bar UI) Quick fixes: - Remove duplicate insertBefore call - Remove duplicate innerHTML assignment Co-Authored-By: Ha AI <1134180104@qq.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the Tauri app against XSS, adds prompt import/export + search capabilities, and attempts to make the embedded service thread more resilient to panics.
Changes:
- Escape user-controlled prompt fields in prompt list rendering; update edit modal to avoid interpolating user data into HTML attributes.
- Add UI controls + Tauri commands for prompt search, JSON export, and JSON import.
- Add a panic-restart loop around
service::run_service(); remove the unused rootwindowscrate dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main_simple.js | Adds prompt search + import/export UI bindings; applies XSS escaping in list rendering and safer modal value assignment. |
| src/main.rs | Adds export_prompts, import_prompts, search_prompts commands; wraps service thread in catch_unwind restart loop. |
| src/index.html | Adds prompt toolbar (search/export/import) and hidden file input for import. |
| Cargo.toml | Removes root windows dependency entry. |
| Cargo.lock | Removes windows 0.52.0 from the root package dependency list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match result { | ||
| Ok(_) => { | ||
| eprintln!("⚠️ [ENGINE] 服务线程正常退出,尝试重启..."); | ||
| } | ||
| Err(e) => { | ||
| let msg = if let Some(s) = e.downcast_ref::<&str>() { | ||
| s.to_string() | ||
| } else if let Some(s) = e.downcast_ref::<String>() { | ||
| s.clone() | ||
| } else { | ||
| "未知错误".to_string() | ||
| }; | ||
| eprintln!("❌ [ENGINE] 服务线程崩溃: {},3秒后自动重启...", msg); | ||
| } | ||
| } | ||
| // 短暂等待后自动重启 | ||
| std::thread::sleep(std::time::Duration::from_secs(3)); | ||
| eprintln!("🔄 [ENGINE] 正在重启服务线程..."); | ||
| } |
There was a problem hiding this comment.
Be careful restarting service::run_service() in-process: service::run_service() currently calls env_logger::init_from_env(...), which panics if the logger is already initialized. After the first panic, the restart loop is likely to immediately panic again during logger init. Consider making logger initialization idempotent (e.g., try_init with ignored error) or moving logger init outside the restart loop.
| fn chrono_now_string() -> String { | ||
| let now = std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .unwrap_or_default(); | ||
| format!("{}", now.as_secs()) | ||
| } |
There was a problem hiding this comment.
chrono_now_string() returns a Unix timestamp in seconds, but the name suggests a formatted datetime string. Consider renaming it to reflect what it returns (e.g., unix_timestamp_secs_string) or changing the implementation to emit an ISO-8601/RFC3339 timestamp if the export format expects a human-readable datetime.
| // Use DOM APIs instead of innerHTML to prevent XSS | ||
| const modal = document.createElement('div'); | ||
| modal.id = 'edit-prompt-modal'; | ||
| modal.className = 'modal-overlay'; | ||
| modal.innerHTML = ` | ||
| <div class="modal-content"> |
There was a problem hiding this comment.
The comment says “Use DOM APIs instead of innerHTML to prevent XSS”, but the implementation still uses modal.innerHTML = ... (it’s just no longer interpolating user data). To avoid confusion, either update the comment to reflect the actual mitigation (no user-data interpolation + property assignment), or build the modal DOM tree without innerHTML.
| for prompt in &export_data.prompts { | ||
| let tags_str = prompt.tags.as_ref().map(|t| t.join(",")); | ||
| let result = conn.execute( | ||
| "INSERT INTO prompts (name, tags, content, content_type, variables_json, app_scopes_json, inject_order, version, updated_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", | ||
| rusqlite::params![ | ||
| prompt.name, | ||
| tags_str, | ||
| prompt.content, |
There was a problem hiding this comment.
import_prompts writes tags back to the DB as a comma-joined string, but the rest of the app expects the tags column to contain JSON (see create_prompt/update_prompt which serde_json::to_string the tag array). This will cause imported prompts to lose tags in get_all_prompts (JSON parse fails) and makes search/export inconsistent. Persist tags using the same JSON encoding used by the other CRUD paths.
| loop { | ||
| let result = std::panic::catch_unwind(|| { | ||
| service::run_service(); | ||
| }); |
There was a problem hiding this comment.
This panic-supervision loop uses catch_unwind, but the workspace Cargo.toml sets [profile.release] panic = "abort", which prevents unwinding in release builds—panics will abort the process and never be caught/restarted. If restart-on-panic is required in production, switch release to panic = "unwind" (and audit the cost), or supervise the service via an external process instead of relying on catch_unwind.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
This PR addresses critical security vulnerabilities and adds key missing features identified during a project analysis.
Security (P0):
loadPrompts()— user-suppliedprompt.name,prompt.content, and tags were interpolated into HTML without escaping. Now uses the existingescapeHtml()helper.showEditPromptModal()— replaced string interpolation of user data intovalue="..."attributes with safe DOM property assignment (el.value = ...).windowscrate (v0.52) from rootCargo.toml. The service module already has its ownwindowsv0.58 dependency.Reliability (P1):
service::run_service()incatch_unwindwith a 3-second auto-restart loop, so the service thread recovers from panics instead of silently dying.Features (P1):
export_promptsTauri command + UI button. Exports all prompts as a JSON file download.import_promptsTauri command + file picker UI. Imports prompts from a previously exported JSON file.search_promptsTauri command + search bar in the prompts panel header. Searches by name, content, and tags via SQLLIKE.Minor fixes:
promptList.innerHTML = promptsHtmlassignmentlogsToolbar.insertBefore(refreshLogsBtn, clearLogsBtn)callReview & Testing Checklist for Human
showEditPromptModal: The modal now creates elements with emptyvalue=""and sets them via DOM properties after appending. Confirm that prompts containing",<,>, and<script>tags render safely in both the list view and the edit modal.catch_unwindcompatibility:service::run_service()must beUnwindSafefor the panic protection to work correctly. The retry loop also has no max retry limit or backoff — if the service panics on startup repeatedly, it will retry every 3 seconds indefinitely.{ jsonData: text }from JS, relying on Tauri 2's automatic camelCase→snake_case conversion to match the Rust parameterjson_data: String. Confirm this works with the project's Tauri invoke path.edition = "2024") and Windows-specific dependencies.Suggested test plan:
<img src=x onerror=alert(1)>— verify it renders as text, not as HTMLNotes
bindFunctionButtonsduplicates the HTML template fromloadPrompts(). A future refactor could extract a sharedrenderPromptCard()helper.search_promptsdoes not escape SQLLIKEwildcards (%,_) in the user query — entering%will match everything. This is low-risk but worth noting.chrono_now_string()returns a Unix timestamp (seconds), not an ISO 8601 datetime, despite the function name suggesting otherwise.Link to Devin session: https://app.devin.ai/sessions/ffc0c35c3461413cae659b426b1007a4
Requested by: @Haaaiawd